Improve error messages when positional arguments are missing#20591
Improve error messages when positional arguments are missing#20591KevinRK29 wants to merge 8 commits intopython:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
test-data/unit/check-functions.test
Outdated
| f("wrong", 123) | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Incompatible arguments for "f"; check for missing arguments |
There was a problem hiding this comment.
I think in this case we can fall back to the pre-existing way of generating messages. It could be useful to show which types are expected vs actual, etc.
test-data/unit/check-functions.test
Outdated
| f(1, b'x', 1) | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Expected "str" for parameter "y"; did you forget argument "y"? |
There was a problem hiding this comment.
This seems inconsistent with other existing messages about argument type mismatches. Can you check these for non-overloads and see if these could be more consistent?
This comment has been minimized.
This comment has been minimized.
…clude implementation details
This comment has been minimized.
This comment has been minimized.
test-data/unit/check-functions.test
Outdated
| f(1, b'x', 1) | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Argument 2 to "f" has incompatible type "bytes"; expected "str" |
There was a problem hiding this comment.
Should this be an error about missing positional argument "y"? I think this is the only error we need to report for this call.
test-data/unit/check-functions.test
Outdated
| f("hello", b'x') | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Argument 1 to "f" has incompatible type "str"; expected "int" |
There was a problem hiding this comment.
Similarly, could we report missing positional argument "x", as the only error?
| f("hello", b'x') | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Missing positional argument "z" in call to "f" |
There was a problem hiding this comment.
In this case it's not clear what the idea error message would be, since the caller accepts *args. One option would be to report missing positional argument x, but maybe it could result in confusing messages in other contexts, so these messages are reasonable. However, I think it would better to show the 'Missing positional argument ...' after the other messages, so that the messages could be shown in a logical order.
| f("hello") | ||
| [builtins fixtures/primitives.pyi] | ||
| [out] | ||
| main:3: error: Missing positional argument "y" in call to "f" |
There was a problem hiding this comment.
Can you report this error after the 'Argument N to ...' errors, so that we'd report the errors in the same order as the arguments are in the call.
| f(1.5, [1, 2, 3]) | ||
| [builtins fixtures/list.pyi] | ||
| [out] | ||
| main:3: error: Missing positional arguments "c", "d" in call to "f" |
There was a problem hiding this comment.
Since there are multiple positional arguments missing, it seems reasonable to not try to align them, so this looks fine, but it would be clearer if the order of the errors would be different (see my other comments).
test-data/unit/check-functions.test
Outdated
| f(1, 1.5, [1, 2, 3], ("a", "b")) | ||
| [builtins fixtures/list.pyi] | ||
| [out] | ||
| main:3: error: Argument 2 to "f" has incompatible type "float"; expected "str" |
There was a problem hiding this comment.
Again, could we report missing positional argument b?
for more information, see https://pre-commit.ci
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
Summary
This PR improves error messages when positional arguments are missing from a function call.
Previously when a user forgets a positional argument, mypy would previously emit multiple type errors because the subsequent arguments would be "shifted" and mismatched with their expected types. Instead of showing multiple type errors, it emits a single consolidated message that suggests which argument might be missing.